GH-47709: [C++][FlightRPC] Return ODBC and data source information #47762

alinaliBQ · 2025-10-08T23:44:47Z

Rationale for this change

ODBC needs to give BI tools information about the driver itself and the data source it is connected to.

What changes are included in this PR?

Implementation of SQLGetinfo to return driver and data source information
Add default values for SQLGetInfo in get_info_cache.cc
Tests

Are these changes tested?

Tested on local MSVC Windows

Are there any user-facing changes?

No

GitHub Issue: [C++] SQLGetInfo implementation #47709

alinaliBQ · 2025-10-08T23:45:06Z

@lidavidm @kou Please review this draft ODBC API PR. The testing folder structure will be in a separate PR

github-actions · 2025-10-08T23:45:10Z

⚠️ GitHub issue #47709 has been automatically assigned in GitHub to PR creator.

cpp/src/arrow/flight/sql/odbc/odbc_api.cc

lidavidm · 2025-10-09T00:03:30Z

cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc

+// Driver Information
+
+TYPED_TEST(FlightSQLODBCTestBase, TestSQLGetInfoActiveEnvironments) {
+  this->Connect();


Could the connect/disconnect be put in a SetUp/TearDown?

Also it might not be wise to add the tests directly to FlightSQLODBCTestBase; could we create a fixture for this test suite specifically (by subclassing)?

yes, added separate test fixture ConnectionInfoTest

kou · 2025-10-09T06:37:54Z

cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc

+TYPED_TEST(FlightSQLODBCTestBase, TestSQLGetInfoDriverName) {
+  this->Connect();
+
+  Validate(this->conn, SQL_DRIVER_NAME, (SQLWCHAR*)L"Arrow Flight ODBC Driver");


Could you use static_cast?

alinaliBQ

Worked on code review comments

cpp/src/arrow/flight/sql/odbc/odbc_api.cc

alinaliBQ · 2025-10-20T23:21:52Z

cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc

+// Driver Information
+
+TYPED_TEST(FlightSQLODBCTestBase, TestSQLGetInfoActiveEnvironments) {
+  this->Connect();


yes, added separate test fixture ConnectionInfoTest

alinaliBQ · 2025-10-20T23:32:53Z

cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc

+TYPED_TEST(FlightSQLODBCTestBase, TestSQLGetInfoDriverName) {
+  this->Connect();
+
+  Validate(this->conn, SQL_DRIVER_NAME, (SQLWCHAR*)L"Arrow Flight ODBC Driver");


cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc

lidavidm

Please rebase!

cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc

lidavidm · 2025-10-31T01:33:52Z

cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc

+void ValidateGreaterThan(SQLHDBC connection, SQLUSMALLINT info_type,
+                         SQLULEN compared_value) {


I do have the same general comment: I think you would be better off creating utility getters (that throw if they fail) and then using standard GTest assertions, instead of making a function for every combination of data type and comparator. (Also, can they be shared across test files?)

Yea when I saw your comment on the SQLStmtAttr PR, I thought of this PR as well since it also has the GreaterThan functions. I was waiting to see if the changes we made for SQLStmtAttr PR #47773 are what you are looking for. This is something we will be working on for sure.

On the other note, I am not sure if sharing certain utility getter across test files would be useful, since for example SQLGetInfo will only be called in connection_info_test.cc and no other test files. We won't be having copies of utility getters in different test files. 👍

Ah, if this is the same test file then never mind that.

@lidavidm I have fixed and added the utility getters, please have another look, thanks!

cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc

alinaliBQ

Addressed code review comments

alinaliBQ · 2025-11-08T00:30:10Z

cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc

+void ValidateGreaterThan(SQLHDBC connection, SQLUSMALLINT info_type,
+                         SQLULEN compared_value) {


@lidavidm I have fixed and added the utility getters, please have another look, thanks!

cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc

Co-Authored-By: rscales <[email protected]> Create connection_info_test.cc Add SQLGetInfo tests Co-authored-by: justing-bq <[email protected]> Co-Authored-By: rscales <[email protected]> Address code review comments Co-authored-by: justing-bq <[email protected]> Work on code review comments Fix build issues with tests Use Utility getters for SQLGetInfo Remove `using List` * fix build issues Add truncation test

alinaliBQ · 2025-11-29T00:36:15Z

Rebased and tested PR changes on local MSVC. PR is ready to be merged

github-actions bot added Component: C++ Component: FlightRPC awaiting review Awaiting review labels Oct 8, 2025

lidavidm reviewed Oct 9, 2025

View reviewed changes

github-actions bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Oct 9, 2025

kou reviewed Oct 9, 2025

View reviewed changes

alinaliBQ mentioned this pull request Oct 10, 2025

GH-46734: [C++] Arrow Flight SQL ODBC layer #46099

Open

alinaliBQ force-pushed the gh-47709-sql-get-info branch from e6f61c6 to 49745a6 Compare October 20, 2025 23:33

alinaliBQ commented Oct 20, 2025

View reviewed changes

github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 20, 2025

lidavidm reviewed Oct 21, 2025

View reviewed changes

cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc Outdated Show resolved Hide resolved

cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc Outdated Show resolved Hide resolved

github-actions bot added awaiting changes Awaiting changes awaiting change review Awaiting change review and removed awaiting change review Awaiting change review awaiting changes Awaiting changes labels Oct 21, 2025

lidavidm reviewed Oct 22, 2025

View reviewed changes

cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc Outdated Show resolved Hide resolved

github-actions bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Oct 22, 2025

alinaliBQ force-pushed the gh-47709-sql-get-info branch from 5452bfd to f7a7386 Compare October 22, 2025 18:50

github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 22, 2025

alinaliBQ requested a review from lidavidm October 22, 2025 23:08

lidavidm reviewed Oct 31, 2025

View reviewed changes

github-actions bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Oct 31, 2025

justing-bq reviewed Nov 5, 2025

View reviewed changes

cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc Outdated Show resolved Hide resolved

cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc Outdated Show resolved Hide resolved

alinaliBQ mentioned this pull request Nov 7, 2025

Develop utility getters for ODBC APIs Bit-Quill/arrow#136

Merged

alinaliBQ force-pushed the gh-47709-sql-get-info branch from f7a7386 to f4aec24 Compare November 8, 2025 00:29

github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Nov 8, 2025

alinaliBQ commented Nov 8, 2025

View reviewed changes

kou changed the title ~~GH-47709 : [C++][FlightRPC] Return ODBC and data source information~~ GH-47709: [C++][FlightRPC] Return ODBC and data source information Nov 9, 2025

lidavidm approved these changes Nov 10, 2025

View reviewed changes

github-actions bot added awaiting merge Awaiting merge and removed awaiting change review Awaiting change review labels Nov 10, 2025

alinaliBQ mentioned this pull request Nov 11, 2025

GH-47707: [C++][FlightRPC] Add tests for descriptor handle allocation #48053

Draft

alinaliBQ force-pushed the gh-47709-sql-get-info branch 4 times, most recently from 605971d to fc711ae Compare November 20, 2025 22:12

alinaliBQ force-pushed the gh-47709-sql-get-info branch from fc711ae to c701849 Compare November 29, 2025 00:35

alinaliBQ marked this pull request as ready for review November 29, 2025 00:36

		void ValidateGreaterThan(SQLHDBC connection, SQLUSMALLINT info_type,
		SQLULEN compared_value) {

GH-47709: [C++][FlightRPC] Return ODBC and data source information #47762

Are you sure you want to change the base?

GH-47709: [C++][FlightRPC] Return ODBC and data source information #47762

Uh oh!

Conversation

alinaliBQ commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

alinaliBQ commented Oct 8, 2025

Uh oh!

github-actions bot commented Oct 8, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alinaliBQ left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lidavidm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

alinaliBQ left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

alinaliBQ commented Nov 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alinaliBQ commented Oct 8, 2025 •

edited

Loading